fix: ingest resolves a schema scope and qualifies non-default schemas (DEV-1758) - #294

Closes DEV-1758.

The bug

duckdb_engine's Inspector.get_table_names(schema=None) returns objects from every schema as bare names. Postgres, MySQL, SQL Server, Snowflake, BigQuery, ClickHouse and SQLite all restrict it to the connection's default schema, so DuckDB is the only exposed Tier-1 dialect. _build_one_model then wrote

sql_table = f"{schema}.{obj.name}" if schema else obj.name   # schema is None

so a non-default-schema object got an unqualified sql_table, the generator emitted FROM reports, and the query failed with table-not-found. Models in the default schema resolved through the search path, which is what made the breakage look partial rather than systemic — the reporter had 3 of 29 models working.

The same schema-blindness hit introspect_utils._get_columns_fallback: with schema is None it issued an information_schema.columns query with no schema filter, so main.reports(a) and s2.reports(b, c) produced one model with columns [a, b, c]. On DuckDB that is not a rare fallback — Inspector.get_columns always raises (pg_catalog.pg_collation does not exist) — so it is the primary column path.

Third defect: cli._run_datasources_create built its DatasourceConfig from name / type / connection_string / description only, using args.schema for the one-shot ingest and then discarding it, while schema_drift._collect_sql_table_diffs reads datasource.schema_name back. So datasources create --schema X --ingest followed by a bare slayer ingest scanned a different schema than validate-models inspected. (MCP's create_datasource already persisted it; the CLI was the odd one out.)

Not literally a regression from #283

Worth stating plainly since the issue title says otherwise: the sql_table assignment is byte-identical before and after #283, and --schema has always qualified correctly. What #283 changed is visibility — views are now ingested by default, and dbt materialises staging models as views, so a dlt+dbt DuckDB file that previously produced a handful of models now produces dozens, most of them in a non-default schema and therefore unqueryable. The issue's v7: sql_table: main.stg_reactions evidence comes from the dbt-import path (slayer/dbt/converter.py passes schema=rm.schema_name), not from bare slayer ingest. The bug is real and fixed as reported; only the framing is off.

Repro, before and after

$ slayer datasources create duckdb:///openfda.duckdb --name openfda_dlt_rest --ingest
$ slayer query '{"source_model":"reports","measures":[{"formula":"*:count","name":"cnt"}]}'

Before: (_duckdb.CatalogException) Table with name reports does not exist! Did you mean "openfda_rest.reports"? [SQL: SELECT COUNT(*) ... FROM reports AS reports]

After:

$ slayer ingest --datasource openfda_dlt_rest
Note: ingested schema 'main' only. Other schemas in this datasource: openfda_rest.
Re-run with --schema openfda_rest, or --all-schemas, to ingest them.        (exit 0)

$ slayer ingest --datasource openfda_dlt_rest --all-schemas
Created: reports (2 columns)          ->  sql_table: openfda_rest.reports
                                          sql_table: in_default

$ slayer query '{"source_model":"reports","measures":[{"formula":"*:count","name":"cnt"}]}'
reports.cnt
2

$ slayer validate-models --datasource openfda_dlt_rest
No drift detected.

What changed

Schema scope. One ingest pass covers one schema unless told otherwise: explicit --schema a,b / --all-schemas (schemas / all_schemas on the Python, REST and MCP surfaces), else datasource.schema_name, else the connection default. Multi-schema is opt-in because it changes what sql_table holds. schema_name is a fallback, never a conflict with an explicit flag; the genuine conflicts (schema+schemas, all_schemas+either) are rejected by one shared helper called from every entry point, so the CLI's add_mutually_exclusive_group is not the only thing holding the line. When exactly one schema is scanned and others exist, the run says which — a hint, not a failure, so the exit code is unchanged.

Two different strings. This is the part that is easy to get backwards, and I got it backwards first — the plan review's initial resolution said normalise tokens to bare, and direct measurement showed the exact inverse. With att_other.duckdb attached as aaa to att_main.duckdb, where shared exists in both:

get_schema_names() on DuckDB returns catalog-qualified tokens always, with or without an ATTACH. So the discovery token is carried exactly as enumerated, end to end, and is_default compares tokens in full — a last-segment comparison is precisely what made att_main.main and other.main both read as the default. The emitted qualifier is a different string: the bare last segment, since the connection's current catalog is already the right one and re-stating it would only break if the datasource were repointed.

test_column_fallback_never_unions_across_catalogs pins the ['m','o'] hazard directly, so the withdrawn rule cannot come back silently.

Both INFORMATION_SCHEMA fallbacks filter on table_catalog. table_schema alone holds the bare name, so a qualified token matched nothing. For the column fallback that meant a model persisted with zero columns and no error; there is deliberately no bare-token retry, since retrying bare is exactly what reintroduces the union. For the PK fallback — which on DuckDB is the path that actually runs, because its Inspector reports an empty constrained_columns even for a declared PRIMARY KEY — it meant every primary key silently dropped, and fan-out safety leans on Column.primary_key. Non-DuckDB dialects carry no catalog segment, so the predicate is never added and their emitted SQL is byte-identical to today's (TestFallbackSqlShape pins that).

With no schema at all the fallback can no longer be narrowed, so instead of unioning every match it groups rows by catalog+schema: one group is used, the default breaks a tie, anything still ambiguous raises. Lowest-sorted-wins was rejected deliberately — it swaps union corruption for wrong-table corruption, which is harder to notice. Per-object isolation turns the raise into a reported skip, so one ambiguous object never aborts the run.

Which objects get qualified. Only non-default schemas, so widening the scan never rewrites models already on disk and one datasource legitimately mixes both forms. A single explicitly-named schema is written verbatim, preserving today's --schema public -> public.orders. A multi-schema list is deliberately not verbatim: listing the default alongside another schema would re-qualify every existing model. --all-schemas means the current catalog only; attached catalogs are dropped loudly, with the exact --schema <catalog>.<schema> invocation that ingests them.

Merging. Re-ingest heals a missing qualifier but never rewrites an existing one. The repair has to participate in the short-circuit and the save gate (same reason source_kind does — a repair usually changes no columns, so a merge that only edits the model_copy(update=...) dict computes the fix and throws it away). Two schemas' same-named tables are never fused into one model: a schema mismatch skips, and — the case a schema comparison alone cannot see, because default-schema models are persisted unqualified — a bare persisted sql_table naming a real default-schema object also skips, rather than being repointed by the heal.

Collisions resolve in one phase over final model names with a 4-key total order (unsanitized beats sanitized, then default schema, then schema name, then object name) rather than successive passes, so the mixed case (s1.a__b sanitizing onto a real s2.a_b) is defined and the outcome never depends on inspector listing order. Losers skip, never suffix.

validate-models derives its schema set from the models being validated — no new flag — and keys the live map on the full <schema_token>.<object> identity plus shorter aliases. A contested alias resolves to the DEFAULT schema's entry, mirroring what the database itself does (FROM orders and FROM main.orders both land in the current catalog); it is dropped only when the default cannot break the tie. This is a data-loss path, not a false-positive nuisance: an unresolvable model becomes a WholeModelDelete, which validate-models --force-clean acts on.

Schema names that arrive from outside — a --schema argument, a persisted schema_name, the bare qualifier read back off a persisted sql_table — are upgraded to the enumerated catalog-qualified token before they reach an Inspector, since a bare token is precisely what sweeps ATTACHed catalogs. What the user typed is kept separately (ResolvedSchema.requested_as) and is what gets emitted, so resolving for discovery can never change the SQL persisted.

One dotted-name splitter (split_sql_table, everything before the final dot) replaces three parsers that disagreed about three-part names, so hand-written Snowflake db.schema.table and BigQuery project.dataset.table stop losing their catalog. That bug exists today, independent of this feature.

No new model field and no migration — the schema lives in sql_table, which is where the generator already reads it. SlayerModel stays at version 8.

Review round 1 (Codex)

Six findings on the first commit, four of them defects I introduced, all reproduced against DuckDB before fixing — see 558c06dc.

• Bare schema names still swept attached catalogs. The token discipline covered the schemas we enumerate but not the ones handed to us. Measured: --schema main with a second catalog attached ingested that catalog's only_in_other and wrote it as main.only_in_other, which does not exist in the current catalog. Fixed by resolve_schema_token + requested_as, described above.
• Dropping every contested alias was itself a data-loss bug. It was meant to avoid an arbitrary winner, but default-schema models are persisted unqualified by design, so the moment another schema gained a same-named table, a legacy sql_table: orders stopped resolving → WholeModelDelete → deleted by --force-clean. Now resolved the way the database resolves it.
• The cross-schema guard failed open. A failed default-schema listing became an empty list, which reads as "no such object" and waved the qualifier repair through, repointing a model at another schema's table. Unknown is now distinct from empty and refuses the merge.
• The PK fallback joined across catalogs. DuckDB names a PK constraint after its column, so a same-shaped table in an attached catalog gets the identical name; joining on constraint name + schema alone returned ['id', 'id']. The join now carries the catalog.

One finding rejected: emitting a 3-part sql_table for an explicitly-named catalog-qualified schema is by design (explicit means verbatim), and verified queryable on DuckDB.

SonarQube: # noqa: CODE — prose is malformed suppression syntax (python:S7632), so the reasons moved to their own line; extracting _index_live_entries also settled the cognitive-complexity finding on _live_schema_for_datasource; one composite test assertion split.

Tests

New tests/test_ingestion_schema_qualification.py: 135 tests over real temp .duckdb files (unit-scoped, the pattern test_cube_js_e2e_duckdb.py already uses — not integration-marked). Written before the implementation; the suite failed to import on IngestSchemaScope until the feature existed.

Full non-integration suite: 7520 passed, 5 skipped, 4 xfailed (7385 before this branch). Ruff clean. DuckDB integration suites re-run green.

Five changes to existing tests, all because the behaviour they pinned genuinely changed:

• test_ingestion.py::test_without_schema asserted "table_schema" not in sql_str and 2-tuple rows. The schema-blind query now selects catalog and schema so it can group instead of union. Updated to assert what actually matters — still parameterized, and no :schema bound.
• test_column_fallback_never_unions_across_catalogs asserted ["m","o"]; ORDER BY ordinal_position says nothing about which of two catalogs sorts first, and DuckDB returned ["o","m"]. Compared sorted.
• Three more after review round 1: the discovery token is now qualified where those tests expected bare (test_requested_schemas_are_marked_explicit, test_multi_returns_objects_tagged_with_their_schema), and a contested alias now resolves instead of missing (test_ambiguous_short_keys_are_dropped_not_overwritten, renamed).

tests/test_ingestion_name_sanitize.py passes unchanged, including its s.table_name == "a__b" bare-label assertion — the schema-qualified skip label is used only when the object set actually spans more than one schema.

The PK regression above was caught by tests/integration/test_ingestion_jaffle_shop.py, not by the unit suite, because nothing in it asserted a primary key on DuckDB. TestPrimaryKeysAreSchemaAware closes that; both of its tests fail when the fix is reverted.

Docs

docs/reference/cli.md (new "Which schemas get ingested" section, both flag tables), docs/concepts/ingestion.md (new "Schema scope" section with the four-surface table), docs/concepts/models.md (when sql_table must be qualified), docs/configuration/datasources.md (schema_name and ingestion), .claude/skills/slayer-models.md, .claude/skills/slayer-overview.md, and a dated DECISIONS.md entry. No new pages, so zensical.toml nav is unchanged.

Known limitations, documented not fixed

• --all-schemas covers the current catalog only. Schemas in an ATTACHed DuckDB catalog are reported as skipped with the explicit invocation that ingests them, rather than guessed at.
• Under multi-schema ingest the YAML mixes qualified and unqualified sql_table values within one datasource. That is the deliberate consequence of not rewriting default-schema models.
• A schema or table whose literal name contains a . stays unrepresentable. Pre-existing.

🤖 Generated with Claude Code